packages/features/flags/features.repository.ts (1)

70-82: Replace Prisma include with select to comply with org-wide guideline.

Per guidelines, avoid include and select only the data you need. Refactor teamFeatures query to use select with nested selects.

Apply this diff:

-    const result = await this.prismaClient.teamFeatures.findMany({
-      where: {
-        teamId,
-      },
-      include: {
-        feature: {
-          select: {
-            slug: true,
-            enabled: true,
-          },
-        },
-      },
-    });
+    const result = await this.prismaClient.teamFeatures.findMany({
+      where: { teamId },
+      select: {
+        feature: {
+          select: {
+            slug: true,
+            enabled: true,
+          },
+        },
+      },
+    });

packages/lib/server/repository/membership.ts (1)

306-315: DI regression: instance method bypasses injected prismaClient.

findUniqueByUserIdAndTeamId is an instance method but calls the global prisma, defeating DI and making tests harder to isolate. Also, select only needed fields per guideline.

Apply this diff:

-  async findUniqueByUserIdAndTeamId({ userId, teamId }: { userId: number; teamId: number }) {
-    return await prisma.membership.findUnique({
-      where: {
-        userId_teamId: {
-          userId,
-          teamId,
-        },
-      },
-    });
-  }
+  async findUniqueByUserIdAndTeamId({ userId, teamId }: { userId: number; teamId: number }) {
+    return await this.prismaClient.membership.findUnique({
+      where: {
+        userId_teamId: {
+          userId,
+          teamId,
+        },
+      },
+      select: {
+        id: true,
+        teamId: true,
+        userId: true,
+        accepted: true,
+        role: true,
+        customRoleId: true,
+      },
+    });
+  }

packages/features/pbac/services/__tests__/permission-check.service.test.ts (1)

697-735: Add a test for org-level explicit permission in checkPermission.

Currently, hasPermission checks explicit permission at team level but only checks manage at org level. Once fixed (see service change), add a test ensuring org-level explicit permission grants access when team-level fails.

Example test to add:

it("should return true when org has explicit permission (no manage)", async () => {
  const membership = { id: 1, teamId: 1, userId: 1, accepted: true, role: "MEMBER" as MembershipRole, customRoleId: "team_role", disableImpersonation: false, createdAt: new Date(), updatedAt: new Date() };

  vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership);
  vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true);
  vi.spyOn(mockRepository, "getMembershipByMembershipId").mockResolvedValueOnce({
    id: membership.id,
    teamId: membership.teamId,
    userId: membership.userId,
    customRoleId: membership.customRoleId,
    team: { parentId: 2 },
  });
  vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce({
    id: 2, teamId: 2, userId: 1, customRoleId: "org_role",
  });

  // Team explicit + manage both fail; org explicit succeeds
  vi.spyOn(mockRepository, "checkRolePermission")
    .mockResolvedValueOnce(false) // team explicit
    .mockResolvedValueOnce(false) // team manage
    .mockResolvedValueOnce(true); // org explicit

  const result = await service.checkPermission({
    userId: 1,
    teamId: 1,
    permission: "eventType.delete",
    fallbackRoles: ["ADMIN", "OWNER"],
  });

  expect(result).toBe(true);
});

packages/features/pbac/services/permission-check.service.ts (1)

217-246: Bug: org-level explicit permission isn’t checked in hasPermission.

Team-level checks verify explicit permission and manage; org-level only checks manage. This is inconsistent with hasPermissions (which checks explicit org permissions first) and can incorrectly deny access when the org grants an explicit permission.

Apply this diff:

   private async hasPermission(query: PermissionCheck, permission: PermissionString): Promise<boolean> {
     const { membership, orgMembership } = await this.getMembership(query);
@@
     // If no team permission, check org-level permissions
     if (orgMembership?.customRoleId) {
-      const [resource] = permission.split(".");
-      const managePermission = `${resource}.manage` as PermissionString;
-      return this.dependencies.repository.checkRolePermission(orgMembership.customRoleId, managePermission);
+      // First check explicit permission at org level
+      const hasOrgPermission = await this.dependencies.repository.checkRolePermission(
+        orgMembership.customRoleId,
+        permission
+      );
+      if (hasOrgPermission) return true;
+
+      // Fallback: check manage at org level
+      const [resource] = permission.split(".");
+      const managePermission = `${resource}.manage` as PermissionString;
+      return this.dependencies.repository.checkRolePermission(orgMembership.customRoleId, managePermission);
     }

packages/features/flags/features.repository.ts (1)

38-41: Consider selecting only needed fields in getAllFeatures.

You only use slug and enabled downstream. To reduce payload and adhere to “only select what you need”, consider selecting those fields. Verify external consumers of getAllFeatures won’t break.

Proposed change:

-    const features = await this.prismaClient.feature.findMany({
-      orderBy: { slug: "asc" },
-      cacheStrategy: { swr: 300, ttl: 300 },
-    });
+    const features = await this.prismaClient.feature.findMany({
+      select: { slug: true, enabled: true },
+      orderBy: { slug: "asc" },
+      cacheStrategy: { swr: 300, ttl: 300 },
+    });

If other callers require additional columns, consider introducing a dedicated method (e.g., getAllFeatureFlagsLight) instead of changing this one.

packages/lib/server/repository/membership.ts (1)

317-320: Static wrapper: add deprecation note to steer callers to DI.

The static delegator is fine for back-compat, but it encourages non-DI usage. Consider annotating as deprecated to nudge migrations.

Suggested JSDoc:

/**
 * @deprecated Prefer creating an instance of MembershipRepository (with injected PrismaClient) and
 * calling instance.findUniqueByUserIdAndTeamId for testability and DI compliance.
 */
static async findUniqueByUserIdAndTeamId(passThroughProps: { userId: number; teamId: number }) {
  const instance = new MembershipRepository();
  return instance.findUniqueByUserIdAndTeamId(passThroughProps);
}

packages/features/pbac/services/__tests__/permission-check.service.test.ts (1)

20-33: Minor: you can configure stubs directly without spyOn for brevity.

Since mockRepository methods are already vi.fn(), prefer setting return values directly (e.g., mockRepository.getUserMemberships!.mockResolvedValueOnce(...)) to reduce boilerplate.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro